feat: use ordered map for always deterministic transaction serialization#364
feat: use ordered map for always deterministic transaction serialization#364JDeuce wants to merge 2 commits intosolana-foundation:mainfrom
Conversation
JDeuce
force-pushed
the
feat/ordered-map-deterministic
branch
from
3364859 to
816bcc9
Compare
|
I dug into
for pair := options.addressTables.Oldest(); pair != nil; pair = pair.Next() {
addressTablePubKey, addressTable := pair.Key, pair.Value
for i, address := range addressTable {
if _, ok := addressLookupKeysMap[address]; ok {
continue
}
addressLookupKeysMap[address] = addressTablePubkeyWithIndex{
addressTable: addressTablePubKey,
index: uint8(i),
}
}
}However, the final for idx, acc := range allKeys {
addressLookupKeyEntry, isPresentedInTables := addressLookupKeysMap[acc.PublicKey]
_, isInvoked := programIDsMap[acc.PublicKey]
if isPresentedInTables && idx != 0 && !acc.IsSigner && !isInvoked {
lookup, _ := lookupsMap.Get(addressLookupKeyEntry.addressTable)
if acc.IsWritable {
lookup.WritableIndexes = append(lookup.WritableIndexes, addressLookupKeyEntry.index)
lookup.Writable = append(lookup.Writable, acc.PublicKey)
} else {
lookup.ReadonlyIndexes = append(lookup.ReadonlyIndexes, addressLookupKeyEntry.index)
lookup.Readonly = append(lookup.Readonly, acc.PublicKey)
}
lookupsMap.Set(addressLookupKeyEntry.addressTable, lookup)
continue
}
}and later: for pair := lookupsMap.Oldest(); pair != nil; pair = pair.Next() {
tablePubKey, l := pair.Key, pair.Value
lookups = append(lookups, MessageAddressTableLookup{
AccountKey: tablePubKey,
WritableIndexes: l.WritableIndexes,
ReadonlyIndexes: l.ReadonlyIndexes,
})
}That means
I verified this locally with a targeted test: given I think the fix is straightforward: when constructing the final
Without that change, the current implementation does not fully deliver the ordering guarantees described by |
|
Yeah this is good feedback. Technically the order was |
|
Great chat! Thank you for your contribution! I saw it covered in #365, closing not, Feel free to open new PR if issues remained. |
3 participants
Summary
Transaction serialization with address lookup tables (ALTs) is non-deterministic today because two internal
maptypes are iterated in random Go map order:options.addressTables— when an address appears in multiple tables, which table "wins" varies between runslookupsMap— the order lookup table entries are appended to the serialized message varies between runsThis is an alternative approach to #344 that solves the same problem by replacing the internal maps with
github.com/wk8/go-ordered-map/v2.Changes
New file:
address_table_map.goHelper functions for converting between
map[PublicKey]PublicKeySlice,[]AddressTableEntry, and the internal*orderedmap.OrderedMap. Centralised here because they are referenced from bothtransaction.goandmessage.go. Theorderedmaptype does not appear in any public API.transaction.gotransactionOptions.addressTablesis now*orderedmap.OrderedMap— preserves insertion orderTransactionAddressTables(map)— signature unchanged; map is converted to an ordered map internally (arbitrary order, same as before)AddressTableEntry— new type:{ TableKey PublicKey; Addresses PublicKeySlice }TransactionAddressTablesSlice([]AddressTableEntry)— new option; caller controls table order and therefore which table takes priority for shared addressesmessage.goMessage.addressTablesfield is now*orderedmap.OrderedMap— nil-safe, preserves insertion orderSetAddressTables(map)— signature unchanged; converts to ordered map internallySetAddressTablesSlice([]AddressTableEntry)— new; ordered setter matchingTransactionAddressTablesSliceGetAddressTables() map— signature unchanged; converts back to plain map on the way outGetAddressTablesSlice() []AddressTableEntry— new; returns tables in insertion orderTests
TestNewTransactionWithAddressLookupTables_Deterministic— 100-iteration test usingTransactionAddressTablesSlicewith two overlapping tables (shared address), asserts byte-identical serialization every runaddress_table_map_test.gocovering round-trips, empty inputs, nil-safety, order preservation, and shared-address priorityCompared to #344
TransactionDeterministicOrdering()TransactionAddressTablesSliceis explicitly ordered;TransactionAddressTablespreserves existing behaviourgithub.com/wk8/go-ordered-map/v2MessageAPI additionsSetAddressTablesSlice,GetAddressTablesSliceThe tradeoff is one new transitive dependency in exchange for a richer, explicit API that makes ordering a first-class concept rather than a hidden sort.